Skip to content

Handle options requests#10

Open
koffeinfrei wants to merge 2 commits intoopenfaas:masterfrom
koffeinfrei:feat/handle-options-requests
Open

Handle options requests#10
koffeinfrei wants to merge 2 commits intoopenfaas:masterfrom
koffeinfrei:feat/handle-options-requests

Conversation

@koffeinfrei
Copy link
Contributor

@koffeinfrei koffeinfrei commented Oct 13, 2019

Add the possibility to handle OPTIONS requests as well in the handler.

This allows to handle e.g. CORS requests, like the following:

class Handler
  def run(body, headers)
    if headers['REQUEST_METHOD'] == 'OPTIONS'
      return [
        [],
        {
          'Allow' => 'POST, GET, OPTIONS',
          'Access-Control-Allow-Methods' => 'POST, GET, OPTIONS',
          'Access-Control-Allow-Origin' => '*',
          'Access-Control-Allow-Headers' => 'Accept'
        }
      ]
    end

    # ....
end

I also extracted the responding logic to a sinatra helper, as the code wasn't very dry.

@koffeinfrei
handle options requests
a204ca0 
Signed-off-by: Alexis Reigel <mail@koffeinfrei.org>
@koffeinfrei
extract response logic to helper
afae342 
Signed-off-by: Alexis Reigel <mail@koffeinfrei.org>
@koffeinfrei koffeinfrei force-pushed the feat/handle-options-requests branch from 5e034f9 to afae342 Compare October 13, 2019 19:18
@alexellis
Copy link
Member

alexellis commented Oct 21, 2019

Is there a linked issue for this?

@koffeinfrei
Copy link
Contributor Author

koffeinfrei commented Oct 21, 2019

Is there a linked issue for this?

Yes: #11

I didn't link it initially as I didn't want to create bias for the discussion in the issue (as I created the MR first, which is not according to you contribution guidelines, as pointed out by you).

@alexellis
Copy link
Member

alexellis commented Oct 21, 2019

Writing the PR first already created a bias.

There is no CORS support in any of the function templates. Having this as a default would go against that, if it's to be added, I would suggest it's made strictly opt-in.

@koffeinfrei
Copy link
Contributor Author

koffeinfrei commented Oct 21, 2019

There is no CORS support in any of the function templates.

Just to make sure we're on the same page: This PR doesn't add CORS support. It just adds a sinatra route for OPTIONS requests, so they are passed to the function as well. What to do with that is still up to the handler function. With this PR GET, POST, PUT, DELETE and OPTIONS requests are processed. Nothing more.

Having said that, do you think passing OPTIONS requests as well could be problematic?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@koffeinfrei @alexellis